Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes for liquid tags #1270

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

fixes for liquid tags #1270

wants to merge 15 commits into from

Conversation

m000
Copy link

@m000 m000 commented Jun 13, 2020

A bunch of fixes for liquid tags:

  • Tests were horribly broken for python2.7. Made several fixes, now works with py27-ipython3 and py37-ipython3. Tests for the notebook tag still fail.
  • SITEURL is prepended to absolute src paths for the img tag. This feature should probably be ported to other media tags.

Plus a new feature:

  • Support for customizing the Liquid Tag block delimiters. This allows mixing with plugins that also use {% and %} to mark their blocks (e.g. jinja2content).

m000 added 13 commits June 13, 2020 21:19
…elimiters.

Previous tag expansion logic: 2 regexes, 4 calls to regex functions,
plus string concatenations.
Simplified tag expansion logic: 1 regex + 1 call to re.sub.

Support for customizing tag delimiters comes handy when used with
plugins with similar tag syntax. E.g. jinja2content.
…the deprecated add().

<rant>
The semantics of add() were inverse to the intuition because they were
implicitly exposing the internals of markdown.util.Registry.

E.g. a priority of ">htmlblock" did not translate to "have greater
priority than htmlblock processor", as one would expect. Instead it
means "when the interal processors array is sorted by priority, the
new processor should have a bigger *index* number than htmlblock".
The tricky point here is that the internal array is sorted in reverse.
So ">htmlblock" actually translates to "use a smaller priority than
htmlblock". Madness!

And instead of fixing the semantics to make sense, it was chosen
to deprecate add() and force us to directly supply the priority to
register(). This is also bad, because we have to grep outside our
codebase to determine the proper priority to achieve the desired effect.
Double the madness!
</rant>
@m000 m000 force-pushed the liquid_tags_fixes branch from 7d0e5d3 to 893429f Compare June 13, 2020 23:31
m000 added 2 commits June 14, 2020 01:35
This is useful when the live site is hosted in a subdirectory
(e.g. http://www.foo.bar/mysite) to avoid having to repeat /mysite
every time.
@m000 m000 force-pushed the liquid_tags_fixes branch from 893429f to 06bc3b0 Compare June 13, 2020 23:35
@kdeldycke
Copy link
Contributor

The liquid_tags plugin has been migrated as a stand-alone plugin at https://github.com/pelican-plugins/liquid-tags . All unittests there have been fixed.

But I guess we need @mirekdlugosz to chime in here and re-evaluate all these changes. @mirekdlugosz, what should we do with this PR?

@mirekdlugosz
Copy link
Contributor

Most of these changes are valuable and I would welcome ported PR in https://github.com/pelican-plugins/liquid-tags

There are only two exceptions:

  • namespaced plugins were introduced in Pelican 4.5.0, and this is also version that dropped support for Python 2.7, so I'm afraid anything related to Python 2 compatibility should be dropped
  • fixes to include_code tag I have applied independently while porting plugin, so they are no longer needed

@m000 is this something you are still interested in? Could you re-apply your changes on top of https://github.com/pelican-plugins/liquid-tags and submit PR there?

@m000
Copy link
Author

m000 commented Feb 8, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants